Skip to content

fixes(#1283): Add documentation for Multer functions #1284

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

arham-sayyed
Copy link

@arham-sayyed arham-sayyed commented Nov 11, 2024

PR Title:

docs: Add documentation for Multer functions

fixes: #1283

#1283

Description:

This pull request adds comprehensive documentation for various Multer functions, improving code readability and helping developers understand the purpose and usage of each function. The following functions have been documented:

  • allowAll
  • Multer.prototype.single
  • Multer.prototype.array
  • Multer.prototype.fields
  • Multer.prototype.none
  • Multer.prototype.any
  • Multer.prototype._makeMiddleware
  • wrappedFileFilter

These additions aim to enhance the clarity of the codebase and provide better insight into the functionality of the Multer library.

Changes:

  • Added JSDoc comments for all relevant functions.
  • Provided detailed descriptions of each function's behavior and parameters.
  • Improved inline documentation to support better developer experience.

Motivation and Context:

Clear documentation helps developers understand how to use the Multer library effectively. This PR aims to improve the usability and maintainability of the code.

Type of Change:

  • Documentation update.

How to Test:

No testing required, as this is purely a documentation update. Review the code for clarity and accuracy of the added comments.

@IamLizu
Copy link
Member

IamLizu commented Dec 27, 2024

This adds a lot of changes, in internal methods as well. While the work is appreciated, I suggest changing the PR to DRAFT so that we do not accidentally land it before its ready as there's so much jsDoc to review.

cc: @UlisesGascon

@arham-sayyed arham-sayyed requested a review from IamLizu January 4, 2025 11:28
@arham-sayyed arham-sayyed marked this pull request as draft April 6, 2025 14:51
@arham-sayyed arham-sayyed marked this pull request as ready for review April 6, 2025 14:52
@UlisesGascon UlisesGascon self-requested a review May 11, 2025 17:16
@UlisesGascon UlisesGascon deleted the branch expressjs:main May 22, 2025 13:47
@UlisesGascon UlisesGascon reopened this May 22, 2025
@UlisesGascon UlisesGascon changed the base branch from master to v2 May 22, 2025 14:13
@bjohansebas bjohansebas requested a review from Copilot June 2, 2025 03:19
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR enhances documentation across Multer by adding detailed JSDoc comments for its storage engines, core middleware, utility classes, and public API methods.

  • Added class- and method-level documentation for memory and disk storage engines.
  • Documented middleware setup logic, error handling, file appender strategies, and counter utilities.
  • Expanded JSDoc on all public Multer APIs (single, array, fields, none, any) with parameter, return, and example annotations.

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

Show a summary per file
File Description
storage/memory.js Documented MemoryStorage, its _handleFile and _removeFile methods, and the factory export.
storage/disk.js Documented getFilename, getDestination, DiskStorage class, its _handleFile/_removeFile, and factory.
lib/remove-uploaded-files.js Documented removeUploadedFiles signature, callback behavior, and recursive removal logic.
lib/multer-error.js Documented MulterError class, its constructor parameters, and inheritance from Error.
lib/make-middleware.js Documented drainStream, makeMiddleware, and internal helpers (done, indicateDone, abortWithError, abortWithCode).
lib/file-appender.js Documented arrayRemove, FileAppender class, and its placeholder/insertion/removal methods.
lib/counter.js Documented Counter class, its event-driven increment/decrement logic, and onceZero.
index.js Documented allowAll, Multer constructor, _makeMiddleware, wrappedFileFilter, and public methods (single, array, fields, none, any, multer).
Comments suppressed due to low confidence (5)

storage/memory.js:3

  • [nitpick] Consider adding an @constructor or @Class annotation to the JSDoc for MemoryStorage to clearly indicate that it is a constructor function.
function MemoryStorage (opts) {}

storage/disk.js:29

  • The os module is referenced here but not imported. Add var os = require('os') at the top of the file to avoid a ReferenceError.
cb(null, os.tmpdir())

lib/remove-uploaded-files.js:13

  • This function implements recursive removal of files and aggregates errors; consider adding unit tests for cases with no files, successful removals, and error aggregation.
function removeUploadedFiles (uploadedFiles, remove, cb) {

lib/multer-error.js:20

  • [nitpick] Add an @extends Error tag in the JSDoc for MulterError to clearly indicate that it inherits from the native Error class.
function MulterError (code, field) {

lib/counter.js:8

  • Counter emits a 'zero' event and manages internal state; consider adding tests to verify event emission when reaching zero and correct increment/decrement behavior.
function Counter () {

@@ -35,6 +91,33 @@ Multer.prototype._makeMiddleware = function (fields, fileStrategy) {
}
})

/**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/**
/**

Copy link
Member

@bjohansebas bjohansebas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve Code Documentation for Enhanced IntelliSense Support
4 participants